[MAIN] [STRATCONN-6317] Replaced tags into properties#835
[MAIN] [STRATCONN-6317] Replaced tags into properties#835AnkitSegment wants to merge 3 commits intomasterfrom
Conversation
| var payload = { | ||
| type: 'event', | ||
| eventName: eventName, | ||
| tags: eventProperties |
There was a problem hiding this comment.
Pull request overview
Updates the Optimizely Analytics.js integration to use properties instead of tags in the Web event payload, aligning with Optimizely Web API changes.
Changes:
- Switched Optimizely Web
trackpayload key fromtagstoproperties. - Updated Optimizely integration tests to assert
propertiesinstead oftags. - Bumped the integration package version from
3.5.1to3.5.2.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| integrations/optimizely/lib/index.js | Changes the outgoing Optimizely event payload key to properties and passes it through to the Fullstack client track call. |
| integrations/optimizely/test/index.test.js | Updates expected pushed payloads and test descriptions to use properties. |
| integrations/optimizely/package.json | Increments the package version for the integration release. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it('should not include revenue on a non Order Completed event if `onlySendRevenueOnOrderCompleted` is enabled', function() { | ||
| analytics.initialize(); | ||
| analytics.track('Order Updated', { | ||
| revenue: 25 | ||
| }); | ||
| analytics.called(window.optimizely.push, { | ||
| type: 'event', | ||
| eventName: 'Order Updated', | ||
| tags: {} | ||
| properties: {} |
There was a problem hiding this comment.
These test descriptions refer to the option name onlySendRevenueOnOrderCompleted, but the integration option is sendRevenueOnlyForOrderCompleted. Updating the wording here would keep the tests aligned with the implementation and reduce confusion.
| it('should repace colons with underscore in eventName', function() { | ||
| analytics.track('event:foo:bar'); | ||
| analytics.called(window.optimizely.push, { | ||
| type: 'event', | ||
| eventName: 'event_foo_bar', | ||
| tags: {} | ||
| properties: {} |
There was a problem hiding this comment.
Typo in test name: repace → replace. This makes the test output/readability clearer.
| var payload = { | ||
| type: 'event', | ||
| eventName: eventName, | ||
| tags: eventProperties | ||
| properties: eventProperties |
There was a problem hiding this comment.
This change switches the Optimizely Web track payload from tags to properties. The in-file docs indicate Classic consumes tags for revenue; if Classic support is still a requirement, consider sending both keys (or gating by snippet/API version) so existing Classic implementations don’t silently drop event metadata.
| var eventName = track.event().replace(/:/g, '_'); // can't have colons so replacing with underscores | ||
| var payload = { | ||
| type: 'event', | ||
| eventName: eventName, | ||
| tags: eventProperties | ||
| properties: eventProperties |
There was a problem hiding this comment.
The docblock for track still refers to sending event data as tags (and mentions Classic consuming a tags object), but the payload now uses properties. Please update the comment (and any in-file references) to match the new payload key to avoid misleading future maintenance.
| it('should not include revenue on a non Order Completed event if `onlySendRevenueOnOrderCompleted` is enabled', function() { | ||
| analytics.initialize(); | ||
| analytics.track('Order Updated', { | ||
| revenue: 25 | ||
| }); | ||
| analytics.called(window.optimizely.push, { | ||
| type: 'event', | ||
| eventName: 'Order Updated', | ||
| tags: {} | ||
| properties: {} |
There was a problem hiding this comment.
These test descriptions refer to the option name onlySendRevenueOnOrderCompleted, but the integration option is sendRevenueOnlyForOrderCompleted (as used throughout the file). Consider updating the wording here to the actual option name to prevent confusion when debugging test failures.
What does this PR do?
Replaced the tags key with properties in alignment with Optimizely’s Web API updates.
JIRA ticket: https://twilio-engineering.atlassian.net/browse/STRATCONN-6317
Testing
Any background context you want to provide?
Is there parity with the server-side/android/iOS integration components (if applicable)?
Does this require a new integration setting? If so, please explain how the new setting works
Links to helpful docs and other external resources